feat: Introduce SentryBreadcrumb class #332
Conversation
|
limbonaut
force-pushed
the
feat/breadcrumb-class
branch
2 times, most recently
from
ebc4e09 to
4e0131b
Compare
limbonaut
force-pushed
the
feat/breadcrumb-class
branch
from
5a82a9a to
b188fa7
Compare
limbonaut
changed the title
limbonaut
requested review from
bitsandfoxes,
bruno-garcia,
jpnurmi and
mujacica
|
|
||
| #include <godot_cpp/classes/ref_counted.hpp> | ||
|
|
||
| using namespace godot; |
There was a problem hiding this comment.
potentially for the future / to be addressed separately: not sure if it has been discussed before, but wouldn't it be better to leave this out of public headers?
There was a problem hiding this comment.
In Godot Engine code base, all the classes live in global namespace. Because we consider supporting compilation as an engine module, I treat this namespace as global as well, at least where it comes to our own class headers that we expose to Godot like SentryBreadcrumb. See #163.
|
|
||
| virtual void set_data(const Dictionary &p_data) override { data = p_data; } | ||
|
|
||
| virtual Ref<SentryTimestamp> get_timestamp() override { return memnew(SentryTimestamp); } |
There was a problem hiding this comment.
does this tend to get called multiple times? would it make sense to allocate the timestamp once as a member if an instance is needed, or return a null ref?
There was a problem hiding this comment.
We don't call it at all, only user would do that possibly within before_breadcrumb (correction: not in Disabled backend). I don't think it's worth optimizing. The only reason why we return a non-null is not to break script code.
For example:
var crumb := SentryBreadcrumb.create()
print(crumb.get_timestamp()) # null access
do_more()The do_more() part would break in noop builds, while being perfectly fine in normal builds.
android_lib/src/main/java/io/sentry/godotplugin/SentryAndroidGodotPlugin.kt
Outdated
Show resolved
Hide resolved
…odotPlugin.kt Co-authored-by: J-P Nurmi <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This PR adds a new breadcrumb API for a cleaner, more intuitive interface. Previously,
add_breadcrumb()method accepted 5 parameters (3 of which were strings), making it confusing to use. The new approach uses a dedicatedSentryBreadcrumbclass:For simple breadcrumbs, you can use a one-liner:
This change provides better type safety, improved readability, and enables future support for the
before_breadcrumbcallback.SentryBreadcrumbclass #338